Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds bugfix for lwip multicast group commands #20133

Conversation

rosahay-silabs
Copy link
Contributor

Problem

  • The function to join the multicast group was not getting invoked due to netif being returned as null from GetNetIfByInterface() which calls netif_get_by_index() with index value 0. The function returns NULL if index 0 is passed as a reference to find the netif from the list of netifs maintained by LwIP.

Change overview

Adds fix for group commands based on UDP multicast.

Testing

No new unit test cases added, since this is part of TC-SC-5.1 and TC-SC-5.2.
Platforms tested:
RPI4 (chip-tool) + EFR32 + (rs9116/wf200)

Depends on #20124

@@ -133,7 +133,7 @@ CHIP_ERROR UDPEndPointImplLwIP::LwIPBindInterface(struct udp_pcb * aUDP, Interfa
InterfaceId UDPEndPointImplLwIP::GetBoundInterface() const
{
#if HAVE_LWIP_UDP_BIND_NETIF
return InterfaceId(netif_get_by_index(mUDP->netif_idx));
return InterfaceId(netif_get_by_index(mUDP->netif_idx + 1));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why "+ 1"?

The intent of the calling code is to do "all interfaces". Why is adding 1 here the right thing?

Copy link
Contributor Author

@rosahay-silabs rosahay-silabs Jun 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mUDP->netif_idx is passing a value of 0, netif_get_by_index returns NULL in that case. Since there is a check for 0, ideally the right thing would be for the netif function to return the interface at 0 position.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The point is, either that 0 is wrong to start with or we are misusing it if it's meant to mean "all interfaces"...

@restyled-io restyled-io bot deleted the branch project-chip:restyled/feature/wifinetworkdiagnostics_impl June 29, 2022 21:47
@restyled-io restyled-io bot closed this Jun 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants